In initramfs, do not prompt if keylocation is "file" #9764

sam-lunt · 2019-12-22T18:08:39Z

If the encryption key is stored in a file, the initramfs should not
prompt for the password. This scenario is possible if the initramfs is
stored on a removable media that is only inserted when booting

Motivation and Context

I placed my /boot partition on flash drive that is only inserted at boot time (I have a home server that normally does not have a monitor attached). The initramfs is built with the key file in the location specified by the keylocation property, so there is no need for the prompt at boot time.

Description

This is a small change to the initramfs-tools script that loads the zfs filesystem. I made the corresponding change to the dracut script as well.

How Has This Been Tested?

I edited the initramfs-tools script for my system, rebuilt the initramfs, and booted successfully. I don't have a system that builds the initramfs using dracut, so that change wasn't tested directly. It's the same logic as the initramfs, however.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.

PrivatePuffin · 2019-12-22T18:44:01Z

@sam-lunt your solution is in fact one of the best for preventing evil maid attacks and yes, ZFS should be able to handle this.

Nice find! :)

Signed-off-by might be case senstive... Interesting... shouldn't be...

contrib/initramfs/scripts/zfs.in

codecov · 2019-12-23T02:50:22Z

Codecov Report

Merging #9764 into master will decrease coverage by <1%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master    #9764    +/-   ##
========================================
- Coverage      80%      80%   -<1%     
========================================
  Files         385      385            
  Lines      121470   121470            
========================================
- Hits        96756    96701    -55     
- Misses      24714    24769    +55

Flag	Coverage Δ
#kernel	`80% <ø> (ø)`	⬆️
#user	`67% <ø> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c587b2c...cb5b875. Read the comment docs.

sam-lunt · 2019-12-23T04:37:01Z

@sam-lunt your solution is in fact one of the best for preventing evil maid attacks and yes, ZFS should be able to handle this.

Nice find! :)

Thanks!

Signed-off-by might be case senstive... Interesting... shouldn't be...

If I need to change the commit message, just let me know.

PrivatePuffin · 2019-12-23T12:00:00Z

@sam-lunt
Ofcourse you need to edit the commit message, because you are failing the style test. because (as I said) it seems the test is case sensitive ;)

sam-lunt · 2019-12-23T14:24:34Z

@Ornias1993

Not sure why, but I read the examples as "Signed-Off-by", so I was confused what the issue was with my message. Anyway, I took a second look and fixed the issue.

contrib/initramfs/scripts/zfs.in

rlaager

I'm marking this approved, as my comment changes fall into "nit" territory. I'd like to see them addressed, but this otherwise seems great to me.

If the encryption key is stored in a file, the initramfs should not prompt for the password. For example, this could be the case if the boot partition is stored on removable media that is only present at boot time Signed-off-by: Sam Lunt <[email protected]>

rlaager

LGTM

PrivatePuffin · 2019-12-24T09:20:20Z

@behlendorf Just a reminder:
It has been flagged as "accepted" but there have been changes pushed that break zfstest.

ghfields · 2019-12-24T13:53:08Z

@behlendorf Just a reminder:
It has been flagged as "accepted" but there have been changes pushed that break zfstest.

Can you be more specific to which failures you believe are from this PR? I am not seeing anything related to encryption lock state.

behlendorf · 2019-12-24T18:02:09Z

@ghfields @Ornias1993 the failures here are unrelated. Sorry, we hit an unexpected snag when updating the CI environment. Once it's sorted out, I'll resubmit this PR for a new run.

PrivatePuffin · 2019-12-24T22:49:11Z

@behlendorf I noticed, forgot to edit my comment, sorry.
@ghfields Please ignore what I wrote....

If the encryption key is stored in a file, the initramfs should not prompt for the password. For example, this could be the case if the boot partition is stored on removable media that is only present at boot time Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Garrett Fields <[email protected]> Reviewed-by: Richard Laager <[email protected]> Reviewed-by: Kjeld Schouten <[email protected]> Signed-off-by: Sam Lunt <[email protected]> Closes openzfs#9764

If the encryption key is stored in a file, the initramfs should not prompt for the password. For example, this could be the case if the boot partition is stored on removable media that is only present at boot time Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Garrett Fields <[email protected]> Reviewed-by: Richard Laager <[email protected]> Reviewed-by: Kjeld Schouten <[email protected]> Signed-off-by: Sam Lunt <[email protected]> Closes #9764

ghfields reviewed Dec 22, 2019

View reviewed changes

contrib/initramfs/scripts/zfs.in Outdated Show resolved Hide resolved

sam-lunt force-pushed the initramfs-headless branch 2 times, most recently from f8ea830 to 13b249a Compare December 23, 2019 04:34

sam-lunt force-pushed the initramfs-headless branch from 13b249a to ba92a8a Compare December 23, 2019 14:24

behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 23, 2019

behlendorf approved these changes Dec 23, 2019

View reviewed changes

behlendorf requested a review from rlaager December 23, 2019 20:03

PrivatePuffin approved these changes Dec 23, 2019

View reviewed changes

rlaager reviewed Dec 23, 2019

View reviewed changes

contrib/initramfs/scripts/zfs.in Outdated Show resolved Hide resolved

rlaager approved these changes Dec 23, 2019

View reviewed changes

sam-lunt force-pushed the initramfs-headless branch from ba92a8a to cb5b875 Compare December 23, 2019 23:07

rlaager approved these changes Dec 23, 2019

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 23, 2019

ghfields approved these changes Dec 24, 2019

View reviewed changes

behlendorf merged commit ad353e2 into openzfs:master Dec 26, 2019

behlendorf mentioned this pull request Dec 28, 2019

zfs-load-key.sh: ${ZFS} is not the zfs binary #9780

Merged

12 tasks

sam-lunt deleted the initramfs-headless branch January 1, 2020 23:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

In initramfs, do not prompt if keylocation is "file" #9764

In initramfs, do not prompt if keylocation is "file" #9764

sam-lunt commented Dec 22, 2019

PrivatePuffin commented Dec 22, 2019 •

edited

Loading

codecov bot commented Dec 23, 2019 •

edited

Loading

sam-lunt commented Dec 23, 2019

PrivatePuffin commented Dec 23, 2019

sam-lunt commented Dec 23, 2019

rlaager left a comment

rlaager left a comment

PrivatePuffin commented Dec 24, 2019

ghfields commented Dec 24, 2019

behlendorf commented Dec 24, 2019 •

edited

Loading

PrivatePuffin commented Dec 24, 2019

In initramfs, do not prompt if keylocation is "file" #9764

In initramfs, do not prompt if keylocation is "file" #9764

Conversation

sam-lunt commented Dec 22, 2019

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

PrivatePuffin commented Dec 22, 2019 • edited Loading

codecov bot commented Dec 23, 2019 • edited Loading

Codecov Report

sam-lunt commented Dec 23, 2019

PrivatePuffin commented Dec 23, 2019

sam-lunt commented Dec 23, 2019

rlaager left a comment

Choose a reason for hiding this comment

rlaager left a comment

Choose a reason for hiding this comment

PrivatePuffin commented Dec 24, 2019

ghfields commented Dec 24, 2019

behlendorf commented Dec 24, 2019 • edited Loading

PrivatePuffin commented Dec 24, 2019

PrivatePuffin commented Dec 22, 2019 •

edited

Loading

codecov bot commented Dec 23, 2019 •

edited

Loading

behlendorf commented Dec 24, 2019 •

edited

Loading